podman: new option --preserve-fd #20866
Conversation
openshift-ci
bot
added
release-note
approved
libpod/oci_conmon_common.go
Outdated
| preserveFDs := ctr.config.PreserveFDs | ||
| preserveFDsMap := make(map[uint]interface{}) |
There was a problem hiding this comment.
should use struct{} over interface{} as it doesn't need to store any info.
But maybe do we even have to use a map here at all?
Given you already iterate anyway I think this would be simpler:
files := make([]*os.File, len(ctr.config.PreserveFDsList))
for _, i := range ctr.config.PreserveFDsList {
if i > preserveFDs {
preserveFDs = i
}
f = os.NewFile()
files[i] = f
}
cmd.ExtraFiles = filesThere was a problem hiding this comment.
never mind this wouldn't work as we do not know the max size without looping over all elements so I the map version is fine with me.
|
WDYT about renaming it to just |
|
I like --preserve-fd, and then we just explain the difference, Few users will ever do this, so it becomes easily descoverable and the options in the man page will be right next to each other. |
giuseppe
force-pushed
the
add-preserve-fds-list
branch
2 times, most recently
from
79f2812 to
f504169
Compare
giuseppe
changed the title
|
renamed |
giuseppe
force-pushed
the
add-preserve-fds-list
branch
from
f504169 to
c772e9d
Compare
|
ready for review |
giuseppe
force-pushed
the
add-preserve-fds-list
branch
2 times, most recently
from
0ea0ff1 to
f4b7317
Compare
openshift-ci
bot
added
the
do-not-merge/hold
giuseppe
force-pushed
the
add-preserve-fds-list
branch
from
f4b7317 to
db35725
Compare
add a new option --preserve-fd that allows to specify a list of FDs to pass down to the container. It is similar to --preserve-fds but it allows to specify a list of FDs instead of the maximum FD number to preserve. --preserve-fd and --preserve-fds are mutually exclusive. It requires crun since runc would complain if any fd below --preserve-fds is not preserved. Closes: containers#20844 Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
add-preserve-fds-list
branch
from
db35725 to
01d397a
Compare
|
can I get another review? |
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, giuseppe, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm I'm sorry for missing your changes yesterday. |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
github-actions
bot
added
the
locked - please file new issue/PR
add a new option --preserve-fd that allows to specify a list of FDs to pass down to the container.
It is similar to --preserve-fds but it allows to specify a list of FDs instead of the maximum FD number to preserve.
--preserve-fd and --preserve-fds are mutually exclusive.
It requires crun since runc would complain if any fd below --preserve-fds is not preserved.
Closes: #20844
Does this PR introduce a user-facing change?